Add snapshot save command (local)#210
Conversation
f6bbfbf to
6a24cdf
Compare
| _ = resp.Body.Close() | ||
| return nil, fmt.Errorf("LocalStack returned status %d", resp.StatusCode) | ||
| } | ||
| return resp.Body, nil |
There was a problem hiding this comment.
question: instead of returning io.ReadCloser can we instead pass the io.Writer destination as argument?
This way we're sure the caller won't forget to close the returned body.
There was a problem hiding this comment.
@anisaoshafi were you able to explore this idea?
There was a problem hiding this comment.
Sorry, I missed this. I gave it a try here 🤞🏼 2bf9e13
|
I've posted a comment[1] in PRO-212 to discuss some potential changes, comments welcome! |
19cab09 to
cc8b494
Compare
e2a83f0 to
b09a963
Compare
36170a5 to
7e45925
Compare
Without this, a failed io.Copy left a corrupt/partial ZIP on disk. The user had no indication the file was incomplete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Passing nil to runLstk inherited the developer's real $HOME, which could write to ~/.config/lstk/lstk.log and the file-keyring fallback. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verify that ErrorEvent is routed to errOut and all other events go to out, and that nil writer arguments fall back safely without panicking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7e45925 to
cc13c98
Compare
cc13c98 to
c680041
Compare
| return nil, f.err | ||
| } | ||
| return io.NopCloser(bytes.NewReader(f.body)), nil | ||
| } |
There was a problem hiding this comment.
nit: can we use gomock to generate this mock exporter?
| // StateExporter retrieves state from the running LocalStack instance. | ||
| type StateExporter interface { | ||
| ExportState(ctx context.Context, host string) (io.ReadCloser, error) | ||
| } |
There was a problem hiding this comment.
nit: the go convention is rather to define an interface where it is used, that would be directly in save.go
| _ = resp.Body.Close() | ||
| return nil, fmt.Errorf("LocalStack returned status %d", resp.StatusCode) | ||
| } | ||
| return resp.Body, nil |
There was a problem hiding this comment.
@anisaoshafi were you able to explore this idea?
| got, err := snapshot.ParseDestination("", now) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, filepath.Join(wd, "snapshot-2026-05-11T21-04-32.zip"), got) | ||
| } |
There was a problem hiding this comment.
nit: can't this test case go as a new row in TestParseDestination?
| _, err := snapshot.ParseDestination(dir, now) | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), "is a directory") | ||
| } |
There was a problem hiding this comment.
question: isn't this test case already covered in TestParseDestination?
|
|
||
| Pass [destination] as an absolute or relative path for the exported file: | ||
|
|
||
| lstk snapshot save # saves to ./snapshot-<YYYY-MM-DDTHH-mm-ss>.zip |
There was a problem hiding this comment.
question: spec says:
Omitting REF generates snapshot-- and writes to CWD.
So it looks like we're missing a hash?
Not sure if it makes sense since it is unlikely to conflict with the timestamp being precise to the second.
There was a problem hiding this comment.
@gtsiolis we agreed to remove the hash, but forgot to remove one reference in the spec. Is that correct?
There was a problem hiding this comment.
Yes, I think we can safely skip it, the timestamp should be quite unique on every generation, right? Plus, system will place a second file next to each other in the rare case you try to generate one in the same second, correct?
snapshot-2026-05-12T20-07-31.zip
snapshot-2026-05-12T20-07-31 (1).zip
There was a problem hiding this comment.
I see we don't handle the double saving on the same second and overwrite the existing one, but sounds like an edge case and ok to leave as is and skip the over-engineering. Could we continue skipping the hash and easily append the (1) on the filename?
Fix AWS-only guard to correctly reject non-AWS emulator configs, align container lookup with the for-loop pattern used in aws.go, route ErrorEvents to stderr via NewPlainSinkSplit, and minor code clarity improvements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
570047a to
e5dfa92
Compare
|
@gtsiolis would really appreciate your UX input when you have time to test this 🙏🏼 |
|
Looking at this now! 👀 |
|
|
||
| Pass [destination] as an absolute or relative path for the exported file: | ||
|
|
||
| lstk snapshot save # saves to ./snapshot-<YYYY-MM-DDTHH-mm-ss>.zip |
There was a problem hiding this comment.
I see we don't handle the double saving on the same second and overwrite the existing one, but sounds like an edge case and ok to leave as is and skip the over-engineering. Could we continue skipping the hash and easily append the (1) on the filename?
| defer func() { | ||
| sink.Emit(output.SpinnerStop()) | ||
| if retErr == nil { | ||
| sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("Snapshot saved to %s", dest)}) |
| defer func() { | ||
| sink.Emit(output.SpinnerStop()) | ||
| if retErr == nil { | ||
| sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("Snapshot saved to %s", dest)}) |
There was a problem hiding this comment.
question: There's some hard text wrapping on long file path. Any quick wins to make this render better on small width terminals? Like ...
- If it's under CWD → ./snapshot-….zip
- If it's under $HOME → ~/snapshot-….zip
- Otherwise leave the absolute path
| Text wrapping for saved snapshot on a small terminal width |
|---|
![]() |



Added a new snapshot subcommand with an initial save action that saves the current LocalStack state to a snapshot file.
Scope for this PR is saving a snapshot locally.
Saving remotely coming next.